Skip to content

test(mbt): Add Model Based Test for Emerald#201

Open
erickpintor wants to merge 2 commits intomainfrom
erick/introduce-mbt-suite
Open

test(mbt): Add Model Based Test for Emerald#201
erickpintor wants to merge 2 commits intomainfrom
erick/introduce-mbt-suite

Conversation

@erickpintor
Copy link
Collaborator

@erickpintor erickpintor commented Jan 29, 2026

This patch connects the newly added Quint specs to Emerald's code via the Quint Connect library.

PS: the best files to start reviewing are probable tests.rs and driver.rs.

@erickpintor erickpintor requested a review from a team as a code owner January 29, 2026 13:26
@erickpintor erickpintor force-pushed the erick/introduce-mbt-suite branch 4 times, most recently from 029e72f to d09bd51 Compare January 29, 2026 21:14
@erickpintor erickpintor changed the base branch from erick/fix-reth-recreate to erick/mbt-reset-timeouts January 29, 2026 21:14
Comment on lines +50 to +68
#[quint_run(
spec = "../../specs/emerald_mbt.qnt",
step = "step_no_failures",
max_samples = 32,
max_steps = 128
)]
fn simulation_no_failures() -> impl Driver {
EmeraldDriver::default()
}

#[quint_run(
spec = "../../specs/emerald_mbt.qnt",
step = "step_with_failures",
max_samples = 32,
max_steps = 128
)]
fn simulation_with_failures() -> impl Driver {
EmeraldDriver::default()
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First uncached run took about 30m in the CI. Let me know if you want to generate fewer traces or reduce the number of steps to minimize test time.

@erickpintor erickpintor requested a review from bugarela January 29, 2026 22:04
Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good from my side!

@erickpintor erickpintor force-pushed the erick/mbt-reset-timeouts branch from 9b46ba5 to 6343e8e Compare February 3, 2026 14:25
@erickpintor erickpintor force-pushed the erick/introduce-mbt-suite branch 4 times, most recently from 58ca2f7 to 1de9af2 Compare February 3, 2026 15:23
@erickpintor erickpintor force-pushed the erick/mbt-reset-timeouts branch from 6343e8e to b9b7e32 Compare February 4, 2026 13:25
@erickpintor erickpintor force-pushed the erick/introduce-mbt-suite branch from 1de9af2 to 54fca10 Compare February 4, 2026 13:28
@erickpintor erickpintor force-pushed the erick/mbt-reset-timeouts branch from b9b7e32 to 133095a Compare February 4, 2026 13:30
Base automatically changed from erick/mbt-reset-timeouts to main February 4, 2026 13:45
@erickpintor erickpintor force-pushed the erick/introduce-mbt-suite branch 2 times, most recently from 9f61e97 to 3d62414 Compare February 4, 2026 14:18
This patch connects the newly added Quint specs to Emerald's code via
the Quint Connect library.
@erickpintor erickpintor force-pushed the erick/introduce-mbt-suite branch from 3d62414 to 805fcc9 Compare February 4, 2026 16:56
let res = Command::new("make")
.env("RETH_NODES", nodes)
.arg(cmd)
.current_dir("../..")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use absolute path here?

you may use CARGO_MANIFEST_DIR.

Comment on lines +45 to +50
if !res.status.success() {
bail!(
"Failed to run make command: {}",
String::from_utf8_lossy(&res.stderr)
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use anyhow::ensure()

impl Runtime {
pub fn new(temp_dir: TempDir) -> Result<Self> {
Ok(Self {
tokio: tokio::runtime::Runtime::new()?,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not TokioRt::new() ?

Comment on lines +116 to +118
- name: Run Forge build
run: forge build
if: steps.filter.outputs.code == 'true'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make mbt-test is already calling forge build.

/// creating one Emerald and Reth instance per [crate::NODES].
pub fn init(&mut self) -> Result<()> {
// Reset the driver
*self = Default::default();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain this line ?

I would rather prefer pub fn reinit(self) -> Result<Self> { ... }

/// - [FailureMode::ProcessRestart]: Restarts Emerald only
pub fn failure(&mut self, node: Node, mode: FailureMode) -> Result<()> {
if let FailureMode::ConsensusTimeout = mode {
// Noop wrt. MBT. The model will follow up with the propoer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Noop wrt. MBT. The model will follow up with the propoer
// Noop wrt. MBT. The model will follow up with the proper

impl EmeraldDriver {
/// Locates the system under test and use the driver's runtime to wait for
/// the execution of the given future.
pub fn perform<'a, F, Fut>(&'a mut self, node: Node, action: F) -> Result<()>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why this is implemented separately.

Comment on lines +18 to +19
let peer_id = PeerId::from_multihash(Default::default())
.map_err(|err| anyhow!("Failed to create peer id: {err:?}"))?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, it's always from the same peer_id?

})
.transpose()?;

if proposal != emerald_proposal {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use anyhow::ensure

Comment on lines +15 to +16
F: FnOnce(&'a mut Sut, &'a mut History) -> Fut,
Fut: Future<Output = Result<()>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't know if it'll fit nicely but there exists AsyncFnOnce.

runs-on: ubuntu-latest
# XXX: Do not block on MBT initially until we gain confidence CI is not
# flaky for any reason.
continue-on-error: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add a timeout here ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants